- 
          
- 
                Notifications
    You must be signed in to change notification settings 
- Fork 4.2k
Fix GL BadDisplay by providing a wl_display handle (WIP on wgpu side too) #20305
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
          
     Closed
      
        
      
            alextechcc
  wants to merge
  22
  commits into
  bevyengine:main
from
alextechcc:myfork/fix_opengl_baddisplay
  
      
      
   
      
    
                
     Closed
            
            Fix GL BadDisplay by providing a wl_display handle (WIP on wgpu side too) #20305
                    alextechcc
  wants to merge
  22
  commits into
  bevyengine:main
from
alextechcc:myfork/fix_opengl_baddisplay
  
      
      
   
              
            Conversation
  
    
      This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
      Learn more about bidirectional Unicode characters
    
  
  
    
    | Closing due to #20358 | 
  
    Sign up for free
    to join this conversation on GitHub.
    Already have an account?
    Sign in to comment
  
      Labels
      
    A-Rendering
  Drawing game state to the screen 
  
    C-Bug
  An unexpected or incorrect behavior 
  
    O-OpenGL
  Specific to the OpenGL render API 
  
    S-Needs-Design
  This issue requires design work to think about how it would best be accomplished 
  Add this suggestion to a batch that can be applied as a single commit.
  This suggestion is invalid because no changes were made to the code.
  Suggestions cannot be applied while the pull request is closed.
  Suggestions cannot be applied while viewing a subset of changes.
  Only one suggestion per line can be applied in a batch.
  Add this suggestion to a batch that can be applied as a single commit.
  Applying suggestions on deleted lines is not supported.
  You must change the existing code in this line in order to create a valid suggestion.
  Outdated suggestions cannot be applied.
  This suggestion has been applied or marked resolved.
  Suggestions cannot be applied from pending reviews.
  Suggestions cannot be applied on multi-line comments.
  Suggestions cannot be applied while the pull request is queued to merge.
  Suggestion cannot be applied right now. Please check back later.
  
    
  
    
NOTE: This is a DRAFT and is based off of several WIP branches/pull requests, including one for WGPU. The commits are ugly, the code is ugly. I hope this can serve as a way to discuss if/how to get a raw display handle over to wgpu when initializing the
wgpuinstance.Objective
BadDisplayafter winit 0.30 on Wayland OpenGL #13923WGPU GL appears to be completely broken on Wayland because if you enumerate adapters before creating a surface (which passes a wl_display handle from winit to the instance) - the GLES backend is forced to create a dummy display connection (https://github.com/gfx-rs/wgpu/blob/trunk/wgpu-hal/src/gles/egl.rs#L901) - and these handles and all resources created with each-other are not compatible, so it is forced to reinitialize its contexts when it is passed the handle (https://github.com/gfx-rs/wgpu/blob/trunk/wgpu-hal/src/gles/egl.rs#L1057), it does this in an unsound way which terminates GL sessions and other ugly side effects.
There is a WIP gfx-rs/wgpu#8012 to allow passing a
RawDisplayHandleto a wgpu instance when constructing it - sidestepping all the ugly context re-initialization - making sure that the contexts are all valid.Currently
bevydoes not seem to be set up to get a valid surface before creating the wgpu instance or requesting adapters. The order seems to be:build()creates the event loop immediately (https://github.com/bevyengine/bevy/blob/main/crates/bevy_winit/src/lib.rs#L136), but does not create windows yet. At this point, it creates aDisplayHandleWrapperwhich would be perfect to throw into the wgpu instance, but unfortunatelybevy_winitdepends onbevy_renderand not the other way around, so it is not possible for it to access this resource.build()queries for a window that already exists to create a surface for, and to use the surface as acompatible_surfaceto pass towgpu. I don't see how this could happen. It misses this, and thus initializes wgpu, doesn't make any surfaces or pass compatible surfaces. https://github.com/bevyengine/bevy/blob/main/crates/bevy_render/src/lib.rs#L384about_to_wait()is called by winit, which creates windowsbevy/crates/bevy_winit/src/state.rs
Line 510 in d1aab7c
resumedevent from Winit. At this point it's far too late to use this window handle.So, the goal of this PR is to figure out a way to throw a
RawDisplayHandleover towgpuwhen initializing the instance so that the egl backend can function.Solution
bevy_windowalso provides RawDisplayHandle (https://github.com/bevyengine/bevy/blob/main/crates/bevy_window/src/raw_handle.rs#L61), and it is depended on by bothbevy_winitandbevy_renderso it's the ideal location to throw theRawDisplayHandle. It seems this is the place thatbevy_rendercan get the contexts it needs, but does not seem to work as intended now due to windows being created way later.bevy_winitalready immediately provides the RawDisplayHandle (https://github.com/bevyengine/bevy/blob/main/crates/bevy_winit/src/lib.rs#L136), but it's just a dependency cycle issue preventingbevy_renderfrom using it immediately. I'm certainly open to suggestions.Testing
WGPU_BACKEND=gl cargo run --features wayland,bevy_render/decoupled_naga --example 2d_gizmos